Add CeloEspressoTimestamp for migration #200

dailinsubjam · 2025-07-25T06:27:17Z

Closes https://app.asana.com/1/1208976916964769/project/1209393353274209/task/1210651926603246?focus=true

Context: So the only change we made to normal op-node is that “reverted transactions from L1 will be skipped”, and this might lead to a chain divergence if different operators start enabling it at different time, then in such case we might need a timestamp for normal op-node.

This PR:

Adds CeloEspressoTimestamp for op-node's migration
The pass of Test6 TestE2eDevNetWithoutAuthenticatingBatches can prove the changes work

This PR does not:

Key places to review:

shenkeyao

LGTM overall, just one question.

shenkeyao · 2025-07-28T23:17:19Z

op-node/rollup/derive/data_source.go

+func isValidBatchTx(tx *types.Transaction, receipt *types.Receipt, l1Signer types.Signer, batchInboxAddr, batcherAddr common.Address, logger log.Logger, celoEspressoTimestamp *uint64) bool {
+	// If CeloEspresso is activated, return false if the transaction is reverted
 	if receipt.Status != types.ReceiptStatusSuccessful {
-		return false
+		logger.Info("tx in inbox with reverted status", "hash", tx.Hash(), "status", receipt.Status)
+		if celoEspressoTimestamp != nil && time.Now().Unix() >= int64(*celoEspressoTimestamp) {
+			logger.Info("tx is dropped since it is reverted")
+			return false
+		}


Is it possible that receipt has a failed status, but not because the transaction is reverted? Double-checking this so we won't mark a transaction as valid if it fails due to a different reason.

That's a great point. OP or Celo does not have this check and we added it, so I think wrapping it in the EspressoEnabled flag adds minimal overhead and gives us extra safety. But worth tagging @QuentinI to confirm that this logic only applies to reverted transactions.

Is it possible that receipt has a failed status, but not because the transaction is reverted?

No, a failed status necessarily means transaction was reverted

op-node/flags/flags.go

QuentinI · 2025-08-01T06:10:37Z

op-node/rollup/derive/data_source.go

+func isValidBatchTx(tx *types.Transaction, receipt *types.Receipt, l1Signer types.Signer, batchInboxAddr, batcherAddr common.Address, logger log.Logger, celoEspressoTimestamp *uint64) bool {
+	// If CeloEspresso is activated, return false if the transaction is reverted
 	if receipt.Status != types.ReceiptStatusSuccessful {
-		return false
+		logger.Info("tx in inbox with reverted status", "hash", tx.Hash(), "status", receipt.Status)
+		if celoEspressoTimestamp != nil && time.Now().Unix() >= int64(*celoEspressoTimestamp) {
+			logger.Info("tx is dropped since it is reverted")
+			return false
+		}


Is it possible that receipt has a failed status, but not because the transaction is reverted?

No, a failed status necessarily means transaction was reverted

op-node/rollup/derive/data_source.go

QuentinI · 2025-08-01T06:16:52Z

op-node/rollup/derive/data_source.go

 	if receipt.Status != types.ReceiptStatusSuccessful {
-		return false
+		logger.Info("tx in inbox with reverted status", "hash", tx.Hash(), "status", receipt.Status)
+		if celoEspressoTimestamp != nil && time.Now().Unix() >= int64(*celoEspressoTimestamp) {


This is not deterministic. We need to be comparing to the block timestamp, not current time.

Updated in 71556a2.

Unfortunately, tx.Time() isn't deterministic either, this is time this particular node has first seen the tx, not a part of consensus. I don't think we can get around pushing thorough the L1 block ref to here and checking against that.

Updated in c0d6688.

dailinsubjam · 2025-08-06T23:10:47Z

After discussing with @Sneh1999, we concluded that while using an activation timestamp doesn’t hurt, it might not actually help either.

Here’s the old proposed migration flow:

Stop the old batcher.
Deploy the new contracts and start the Espresso dev node.
Start the new batcher, which should connect to Espresso and begin submitting attestations.
Restart the op-nodes. All op-nodes will switch to the new version at the same timestamp. By this point, the necessary contracts will already be deployed.

The key point is: op-nodes should switch to the new derivation pipeline before the new batcher is integrated. As long as this order is maintained, the check will be triggered once we switch the batcher, and the system remains safe without the activation timestamp.

So, the activation timestamp doesn’t provide additional guarantees—unless the check we add can also filter out any transactions pre-Espresso.

The most likely order is 2 → 4 → 1 → 3, to minimize the gap between stopping the old batcher and starting the new one.

dailinsubjam · 2025-08-07T00:04:57Z

Pasting @jbearer 's comments and we might use this instead, will also document in the book:

I think the canonical way to handle this type of migration is to have a flag in the chain config like espressoEnabled. This can be set to true via an onchain upgrade action (the chain config is just an L1 smart contract). At any point in the derivation pipeline, we have a well defined L1Origin, so we can get the chain config at that L1 block number (there should already be code for this) and then just gate the revert check with if cfg.espressoEnabled.
I believe it does load the latest chain config every time there is a new L1 origin.
The main advantage of this over the timestamp is that the timestamp approach would be inventing a whole new mechanism to trigger an upgrade, whereas this mechanism (changing some variable in the chain config contract) is already implemented I think.

dailinsubjam added 2 commits July 25, 2025 06:24

add CeloEspressoTimestamp

07e50fc

remove unneeded change

ae1317f

dailinsubjam mentioned this pull request Jul 25, 2025

Add an activation timestamp config for migration to Espresso locally #186

Closed

fix tests

d53a44f

dailinsubjam requested review from QuentinI, philippecamacho and shenkeyao July 25, 2025 17:12

shenkeyao reviewed Jul 28, 2025

View reviewed changes

QuentinI requested changes Aug 1, 2025

View reviewed changes

comment

71556a2

dailinsubjam requested review from QuentinI and shenkeyao August 1, 2025 17:19

dailinsubjam added 3 commits August 1, 2025 21:38

add L1blockref for timestamp check

c0d6688

adapt derive directory

02ac426

format

a9facec

Add CeloEspressoTimestamp for migration #200

Are you sure you want to change the base?

Add CeloEspressoTimestamp for migration #200

Uh oh!

Conversation

dailinsubjam commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR:

This PR does not:

Key places to review:

Uh oh!

shenkeyao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dailinsubjam commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dailinsubjam commented Aug 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dailinsubjam commented Jul 25, 2025 •

edited

Loading

dailinsubjam commented Aug 6, 2025 •

edited

Loading